web: safely handle immutable globals #779 #780
Conversation
JerwuQu
changed the title
There was a problem hiding this comment.
Thanks @JerwuQu!
What do you guage the impact of immutable globals silently failing is? Is it worth debug.erroring to this Pr/issue chain for early adopters writing GC games?
|
This alone won't affect GC games, and I think logging these exceptions won't be useful since they'd happen every tick. I haven't read up on the WASM GC proposal in detail but afaik GC objects are stored outside of the main WASM memory and it would therefore be hard to get GC games working with netplay at all (unless JavaScript has a way to fully manipulate the GC heap, which is pretty unlikely). Unless I'm mistaken about the above or we go all in on very complex WASM patching, we would likely need to change how netplay works and freeze the whole game at start and then have a "start multiplayer" button which locks in all current players and prevents any new from joining. We'd also have to remove rollback netcode completely since that depends on being able to swap memories. Not desirable! In short: GC games will likely be broken in netplay, but it won't be because of globals. |
|
Thanks! I think it's a fine solution, and agree we probably don't want to get crazy with opcode parsing. |
See #779
Though this looks a bit ugly, it beats the alternative.
Explanation:
JavaScript doesn't support checking directly whether a
Globalobject is mutable or not, and parsing enough WASM to determine if each global is mutable requires being able to parse WASM expressions, meaning all possible opcodes permitted in initialization and their parameters. I gave it a quick attempt still, but the amount of opcodes that you have support depends on WASM extensions (e.g. globals using reference types and vector types add more opcodes you need to be able to handle in expressions), and simply adding the ones available today doesn't feel future-proof.I'm also fine with reverting the global patching completely, though it would be a bit unfortunate.
Another alternative is using a proper WASM parsing library, but that increases bundle size instead, and would still need updating if a new WASM extension is ever made.
The
DEV_NETPLAYaddition is for it to use thehttp://localhost:3000/?netplay=xyzURL rather than wasm4.org when enabled, which eliminated some domain issues I was having otherwise. I figured I'll look into that issue in a separate PR.